Cache DocumentDiagnosticAnalyzer results by version stamp to avoid redundant typecheck on unchanged documents - #20121
Conversation
…recomputing diagnostics when document/project version is unchanged
xperiandri
changed the title
DocumentDiagnosticAnalyzer results by version stamp to avoid redundant typecheck on unchanged documents
github-actions
Bot
added
the
AI-Tooling-Check-Scanned-Clean
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.
| document.Project.Solution.GetFSharpExtensionConfig().ShouldProduceDiagnostics() | ||
|
|
||
| static let cache = | ||
| ConcurrentDictionary<struct (DocumentId * DiagnosticsType), VersionStamp * VersionStamp * ImmutableArray<Diagnostic>>() |
There was a problem hiding this comment.
Unbounded static cache / memory retention. This static ConcurrentDictionary is keyed by struct (DocumentId * DiagnosticsType) and has no eviction path. Edits to the same document overwrite their own key, so growth is bounded by the number of distinct documents ever opened (×2 for Syntax/Semantic) — but entries for documents that are closed or removed from the solution are never freed and persist for the entire lifetime of the devenv.exe process. Each retained entry holds an ImmutableArray<Diagnostic> (with Location/file-path data), so in a long-running session over a large solution this is steady, unbounded retention.
Consider evicting on workspace changes — e.g. subscribe to Workspace.WorkspaceChanged and drop entries on DocumentRemoved/DocumentRemovedFromSolution (and optionally when a document is closed), or cap the cache size. The referenced emitCache in FSharpProjectOptionsManager.fs is keyed by project reference and thus has far smaller cardinality than per-document diagnostics, so the retention surface here is larger.
2 participants
Fixes #20120
Summary
FSharpDocumentDiagnosticAnalyzer.GetDiagnosticspreviously recomputed syntax/semantic diagnostics (parse, typecheck,UnusedParentheses) on every crawler pass, even when the document text and project state had not changed since the last computation.This adds a version-stamp-aware cache keyed by
struct (DocumentId * DiagnosticsType), storing the last computed(textVersion, projectVersion, ImmutableArray<Diagnostic>)tuple:Syntaxdiagnostics, only the documenttextVersionis tracked (projectVersionusesVersionStamp.Default).Semanticdiagnostics, bothtextVersionanddocument.Project.GetDependentVersionAsync()are tracked.UnnecessaryParenthesesDiagnosticAnalyzerwork entirely.This mirrors the versioned-cache pattern used for referenced-project compilation emission in
FSharpProjectOptionsManager.fs(emitCache).Testing
dotnet build vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj -c Debug— succeeded.Build.cmd -c Debug -deployExtensions) and validated with a CPU trace ofdevenv.exe.